[FLINK-39284][table] `CREATE OR ALTER MATERIALIZED TABLE` doesn't respect schema of existing table by snuyanzin · Pull Request #27807 · apache/flink

snuyanzin · 2026-03-22T22:31:25Z

What is the purpose of the change

A number of cases like

CREATE MATERIALIZED TABLE mt1 (
   id INT, t TIMESTAMP_LTZ(3)
)
AS SELECT 1 as id, current_timestamp as t

and then

CREATE OR ALTER MATERIALIZED TABLE mt1 (
   id INT, t TIMESTAMP_LTZ(3),
   WATERMARK FOR t as current_timestamp - INTERVAL '5' SECOND
)
AS SELECT 1 as id, current_timestamp as t

it will ignore watermark (all cases like create/modify/drop)
same for constraint
also similar issue for column comments

Brief change log

converter and tests

Verifying this change

new tests introduced

Does this pull request potentially affect one of the following parts:

Dependencies (does it add or upgrade a dependency): ( no)
The public API, i.e., is any changed class annotated with @Public(Evolving): ( no)
The serializers: ( no)
The runtime per-record code paths (performance sensitive): (no)
Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (no )
The S3 file system connector: (no )

Documentation

Does this pull request introduce a new feature? (no)
If yes, how is the feature documented? (not applicable)

…ABLE`

…t constraint changes

… MATERIALIZED TABLE` similar to `CREATE`

…t watermark changes

flinkbot · 2026-03-22T22:38:08Z

CI report:

5a05a62 Azure: SUCCESS

Bot commands

The @flinkbot bot supports the following commands:

@flinkbot run azure re-run the last Azure build

…t column comment change

raminqaf · 2026-03-23T08:54:45Z

...table-planner/src/main/java/org/apache/flink/table/planner/utils/MaterializedTableUtils.java

+                if (!oldColumn.getName().equals(newColumn.getName())
+                        || !LogicalTypeCasts.supportsImplicitCast(
+                                oldColumn.getDataType().getLogicalType(),
+                                newColumn.getDataType().getLogicalType())) {


What happens here if we change we change the type of the column here? From CHAR(1) -> CHAR(4)

raminqaf · 2026-03-23T09:04:17Z

...-api-java/src/main/java/org/apache/flink/table/operations/ddl/AlterTableChangeOperation.java

+                    modifyColumnComment.getNewComment() == null
+                            ? ""
+                            : modifyColumnComment.getNewComment());


Shouldn't we avoid MODIFY COMMENT if the comment is null, instead of returning:

MODIFY 'id' COMMENT ''

no, this type modification means it is removed
since there is no a dedicated table change for that

Got it thanks!

raminqaf · 2026-03-23T09:07:09Z

...ble/planner/operations/SqlNodeToOperationSqlCreateOrAlterMaterializedTableConverterTest.java

Thanks for the new test setup. I noticed that some cases are not tests:

Adding a constraint, for the case that oldConstraint == null

Unchanged table (zero chagnes)

It would be interesting to have a test that has a schema with only watermark/constraint, no regular columns

It would be interesting to have a test that has a schema with only watermark/constraint, no regular columns

what do you mean by that?

if there is no explicit columns in schema they will be derived from query and there are already such tests here

If other tests are covering it, I am fine with it. Thanks for the explanation!

AHeise

Thank you very much for all the fixes. Had some minor things.

AHeise · 2026-03-23T09:47:04Z

...ble/planner/operations/SqlNodeToOperationSqlCreateOrAlterMaterializedTableConverterTest.java

+        assertThat(operation.asSummaryString())
+                .isEqualTo(
+                        "CREATE OR ALTER MATERIALIZED TABLE builtin.default.mt\n"
+                                + "  DROP CONSTRAINT ct1,\n"


These extra space on the first modification look unintended. Can you please double-check?

AHeise · 2026-03-23T09:48:52Z

...table-planner/src/main/java/org/apache/flink/table/planner/utils/MaterializedTableUtils.java

+                        || !LogicalTypeCasts.supportsImplicitCast(
+                                oldColumn.getDataType().getLogicalType(),
+                                newColumn.getDataType().getLogicalType())) {
+                    throw new ValidationException(


Can we move the exception also to the operation and just issue a TableChange.ModifyPhysicalColumnType?

AHeise · 2026-03-23T09:49:56Z

...table-planner/src/main/java/org/apache/flink/table/planner/utils/MaterializedTableUtils.java

+                                            + "Column mismatch at position %d: Original column is [%s], but new column is [%s].",
+                                    i + 1, oldColumn, newColumn));
+                }
+                if (!Objects.equals(oldColumn.getComment(), newColumn.getComment())) {


Since we often treat comment = '' == comment = null, shall we normalize here? Wrap comments in some StringUtil.emptyIfNull.

AHeise · 2026-03-23T09:52:05Z

...-api-java/src/main/java/org/apache/flink/table/operations/ddl/AlterTableChangeOperation.java

                    "  MODIFY %s COMMENT '%s'",
                    EncodingUtils.escapeIdentifier(modifyColumnComment.getNewColumn().getName()),
-                    modifyColumnComment.getNewComment());
+                    modifyColumnComment.getNewComment() == null


2-3 commits before, we already had some changes to the comment logic. Can you double-check if the commits are correctly cut?

AHeise · 2026-03-23T09:53:27Z

...ble/planner/operations/SqlNodeToOperationSqlCreateOrAlterMaterializedTableConverterTest.java

                "CREATE MATERIALIZED TABLE mt1 (\n"
                        + "   id INT, t TIMESTAMP_LTZ(3),\n"
-                        + "   WATERMARK FOR t as current_timestamp - INTERVAL '5' SECOND"
+                        + "   WATERMARK FOR t as current_timestamp - INTERVAL '5' SECOND\n"


This change should probably go into previous commit.

AHeise · 2026-03-23T09:53:59Z

...ble/planner/operations/SqlNodeToOperationSqlCreateOrAlterMaterializedTableConverterTest.java

    }

+    @Test
+    void testCreateOrAlterMaterializedTableWithNewConstraint()


Please squash into respective commit.

snuyanzin added 4 commits March 22, 2026 23:25

[FLINK-39284][table] Extract test for `CREATE OR ALTER MATERIALIZED T…

a44fbe7

…ABLE`

[FLINK-39284][table] Make CREATE OR ALTER MATERIALIZED TABLE respec…

7792e6c

…t constraint changes

[FLINK-39284][table] Nullify types for columns while `CREATE OR ALTER…

dce1236

… MATERIALIZED TABLE` similar to `CREATE`

[FLINK-39284][table] Make CREATE OR ALTER MATERIALIZED TABLE respec…

0bf8818

…t watermark changes

[FLINK-39284][table] Make CREATE OR ALTER MATERIALIZED TABLE respec…

add418d

…t column comment change

raminqaf reviewed Mar 23, 2026

View reviewed changes

snuyanzin added 2 commits March 23, 2026 10:15

Add test with new constraint

37d0f1d

Add test with no table changes

5a05a62

AHeise reviewed Mar 23, 2026

View reviewed changes

github-actions bot added the community-reviewed PR has been reviewed by the community. label Mar 23, 2026

Conversation

snuyanzin commented Mar 22, 2026

What is the purpose of the change

Brief change log

Verifying this change

Does this pull request potentially affect one of the following parts:

Documentation

Uh oh!

flinkbot commented Mar 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CI report:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AHeise left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

flinkbot commented Mar 22, 2026 •

edited

Loading